-
-
Notifications
You must be signed in to change notification settings - Fork 59
fix: Better consideration of the negated matcher in waitUntil
#1983
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix: Better consideration of the negated matcher in waitUntil
#1983
Conversation
Might need a second opinion here, since now when using |
|
@dprevost-LMI merged some PRs, mind updating this branch? |
3c7baa4 to
da0b63e
Compare
|
@christian-bromann Done |
|
Let's rebase, happy to merge after. |
|
@christian-bromann ready again |
|
🤔 When using |
|
|
||
| }) | ||
|
|
||
| describe('Matchers pass when using `.not`', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add integration of .not with Jest's expect lib to ensure we process them correctly
|
|
||
| }) | ||
|
|
||
| describe('Matcher eventually passing', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test around the fix I made to ensure consistent behaviour between a matcher and its negated case when we wait for a given result.
| await expect(() => expectLib(el).not.toBeDisplayed({ wait: 1 })).rejects.toThrow(`\ | ||
| Expect $(\`selector\`) not to be displayed | ||
| Expected [not]: "not displayed" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Failure message fixed in #1987
a11d10f to
e68c889
Compare
fix UT following isNot fixes in waitUntil
chore: ensure using `expect(result.message()).toContain('not')`
chore: include `toHaveTitle` test on main + increase coverage %
Add toHaveTitle on main so it will be easier to "compare" with multi-remote support changes
Squash commits
fix develop merge quirks
fix incorrect test and title
Remove any and fix mocks
When using `.not` result.pass must be false for the test to pass
it is the layer of Jest that inverted the pass pass =false to true when using `.not`
Bring back isNot pass=false for success and isNot pass=true for failures
Add integrated test to ensure isNot behavior + matcher consistency
- Add test on top of Jest's expect lib to ensure that that layer works with our matcher and `.not` correctly process.
- Add test to cover the previous inconsistency of similarly case passing with and without `.not`
Make test a bit faster
Review test for null/undefined cases in toHaveElementPropert
9bed9e3 to
5811396
Compare
Fixes #1982.
Now:
.not, the result returned bywaitUntilis still true for success and false for failure.not, the result returned bywaitUntilis now always false for success and true for failure since Jest's expect framework inverts the boolean later on.isNotanymore since we do not want to rely on it to fail fast, instead we wait for the condition to be true (or timeout) and test the same really has withoutisNot.notbehaviour is conserved over time.not, this can be mitigated by passing awaitvalue in the option parameterBonus: Fix
toHaveElementPropertytyping in thed.tsfileWhy do we need to wait?
Example: If we have a wait of 10 sec and the text becomes 'GOOD' at 5 sec
Then
will pass after waiting 5 sec
Also, with today's bug, if you do
It will also pass because, on the first attempt, it is effectively false.
Having both above conditions pass for the same scenario does not make sense.
By waiting, we ensure we have the 'real' fetched value and negate that 'reality.'
Code representing the above example